feat(server-utils): Add tracingChannel-to-span binding#21641
Conversation
8c4ca47 to
61a560f
Compare
size-limit report 📦
|
2fbdda3 to
3cb7959
Compare
| // Presence checks because caller can return `undefined` result or throw a falsy value. | ||
| if ('error' in data || 'result' in data) { | ||
| endBoundSpan(data, beforeSpanEnd); | ||
| } |
There was a problem hiding this comment.
Sync end closes streaming spans early
Medium Severity
In auto lifecycle mode, the end handler ends the bound span whenever 'result' in data. Orchestrion-style channels (documented for mysql in this repo) can publish end with a result that is only an in-flight handle (e.g. a streamable Query emitter) while the operation continues with no asyncEnd. The span is ended at synchronous end instead of when the work actually finishes.
Reviewed by Cursor Bugbot for commit 3cb7959. Configure here.
Add `bindTracingChannelToSpan` to generalize Node `diagnostics_channel` TracingChannel instrumentation onto Sentry spans. It binds the active span into async context on `start` and, in the default `auto` lifecycle, ends the span on the canonical terminal event: `end` for synchronous calls (detected via presence of `result`/`error` on the context object) and `asyncEnd` for async calls, recording exceptions on `error`. Backed by a new `getTracingChannelBinding` async context strategy hook wired through core, node-core, opentelemetry, cloudflare, and deno.
…oSpan
`bindTracingChannelToSpan` now returns a `TracingChannelBindingHandle`
`{ channel, unbind }` instead of the channel directly. `unbind()`
unsubscribes the auto lifecycle handlers and unbinds the start store
(idempotent; a no-op when no async context binding is available), so
callers can detach a binding — useful for teardown and test isolation.
3cb7959 to
e34e28a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e34e28a. Configure here.
| // Idempotent. | ||
| expect(() => unbind()).not.toThrow(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Feat PR lacks integration test
Low Severity
This feature adds bindTracingChannelToSpan and async-context binding hooks but only adds Vitest unit coverage under packages/server-utils/test. Review guidelines for feat PRs expect at least one integration or E2E test exercising the new behavior in a runtime scenario.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit e34e28a. Configure here.
…ToSpan + captureError callback `getSpan` may now return `undefined` to opt a channel payload out entirely: nothing is bound, no span is tracked, and the active context is left untouched so nested operations keep parenting to the enclosing span. This lets a single channel carry events that should reuse the enclosing span (e.g. an agent loop's per-step events) without ending it prematurely. The `error` handler likewise no-ops when no span was bound. Also folds in the `captureError` callback form: pass a function to set the ExclusiveEventHintOrCaptureContext on the captured error, so integrations can supply their own mechanism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>


This abstracts away our tracing channels consumption into a utility that:
This util can be consumed uniformly across all of our server-side runtimes, I will create a stacked PR that refactors our code to use it, showcasing Nitro, Redis, and Deno adoption of that util.
API
Lifecycle Management
After testing across several node releases (20 -> 26), I locked down the strategy that we can rely on to make this reliable and predictable.
startlifecycle event, this is emitted always and is reliable.errorFor capturing exceptions and setting error span status, always reliable.endis emitted:erroris present on the context object, end the span immediatly, there won’t be any async events coming up, the function threw in the sync part, so there is no async part to execute.resultis present on the context object, end the span immediately, there won’t be any async events coming up. The function returned in the sync part or is sync trace itself. UseinorhasOwnProperty, Do not useundefinedchecks as the function can return nothing.asyncStartis emitted: Do nothing, this event has no value for us.asyncEndis emitted: Just end the span.